Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove added parent directory from certs path #281

Closed
wants to merge 4 commits into from

Conversation

yorickvanzweeden
Copy link

See #279

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 94.618% when pulling 94a9017 on yorickvanzweeden:master into 2450dec on liampauling:master.

@yorickvanzweeden
Copy link
Author

@liampauling Seems that coverage has declined by 0.0002% and thus failing. Could you take a look at the PR and let me know what you think?

@@ -156,9 +156,8 @@ def cert(self) -> list:
return self.cert_files

certs = self.certs or "/certs/"
Copy link
Member

@liampauling liampauling Mar 6, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am thinking that maybe we should update the class initialisation as well and remove this line and replace with in the init:

certs: str = os.path.join(“certs”)

@@ -199,7 +199,7 @@ def test_client_certs_mocked(self, mock_listdir):
[".DS_Store", "client-2048.crt", "client-2048.key"]
)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It would be good to add another test that confirms that certs=“/tmp” is correct

@liampauling
Copy link
Member

@liampauling Seems that coverage has declined by 0.0002% and thus failing. Could you take a look at the PR and let me know what you think?

Yeah don’t worry about this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants